Swift: Update the new metatype sinks#21905
Merged
Merged
Conversation
jketema
approved these changes
May 29, 2026
Contributor
jketema
left a comment
jketema
left a comment
There was a problem hiding this comment.
Thanks. This looks very reasonable.
Contributor
Are you sure it's a performance regression and not just the general performance flakiness of the macOS runners? |
Contributor
Author
|
I did a repeat run and it does indeed look like flakiness. |
Contributor
|
@geoffw0 Note that I already approved |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Swift weak-hashing security extension sinks to identify metatype qualifiers structurally through TypeDecl.getInterfaceType() rather than by matching ".Type" in type names.
Changes:
- Reworks CryptoKit metatype sink matching for weak sensitive-data hashing algorithms.
- Applies the same metatype matching approach for weak password hashing algorithms.
- Keeps the existing call target and argument matching behavior intact.
Show a summary per file
| File | Description |
|---|---|
swift/ql/lib/codeql/swift/security/WeakSensitiveDataHashingExtensions.qll |
Updates Insecure.MD5/Insecure.SHA1 metatype sink qualifier matching. |
swift/ql/lib/codeql/swift/security/WeakPasswordHashingExtensions.qll |
Updates SHA256/SHA384/SHA512 metatype sink qualifier matching. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Update the new metatype sinks to use
.getInterfaceType()rather than relying on name matching (".Type").@jketema this is the best I could do without spending much too long on this. I had a good look at how to generalize this to all Swift MaD sources and sinks - the issue being that
interpretElement0only cares about functions, not calls, so there's no access to the call qualifier as the current solution uses. A wider fix might need to be deeper in the dataflow wiring (perhaps somewhere inDataFlowPrivate.qll).I do still need to look into the performance regression I saw on the last PR, whether it's real and whether this change makes it better or worse.